Vector length diving + Farkas diving#1401
Conversation
…udo_costs, worker and diving_heuristics. Signed-off-by: Nicolas L. Guidotti <nguidotti@nvidia.com>
…derabbit comments. Signed-off-by: Nicolas L. Guidotti <nguidotti@nvidia.com>
Signed-off-by: Nicolas L. Guidotti <nguidotti@nvidia.com>
📝 WalkthroughWalkthroughThis PR introduces two new MIP diving heuristics—Farkas and vector-length strategies—by adding a templated configuration struct, enumerating new search strategies, implementing the selection logic, instrumenting the LP problem with coefficient magnitudes, integrating strategy dispatch into branch-and-bound, and registering runtime parameters in solver settings. ChangesDiving Heuristics Feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
cpp/src/branch_and_bound/pseudo_costs.hpp (1)
32-37: 🏗️ Heavy liftForward declarations here conflict with the repository header policy.
This header now depends on forward declarations for
branch_and_bound_worker_t/branch_and_bound_stats_t. Please refactor the include graph (for example by splitting strategy-enable utilities into a lighter header) so this header can include concrete dependencies instead of forward-declaring them.As per coding guidelines: "Avoid forward declarations in favor of including headers in C++."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cpp/src/branch_and_bound/pseudo_costs.hpp` around lines 32 - 37, The forward declarations of branch_and_bound_worker_t and branch_and_bound_stats_t in pseudo_costs.hpp violate the header policy; replace these forward declarations by including the concrete headers that define branch_and_bound_worker_t and branch_and_bound_stats_t (or refactor by extracting the minimal strategy utilities used by pseudo_costs.hpp into a lightweight header that both this file and the full worker/stats headers can include). Update pseudo_costs.hpp to `#include` the new/lightweight header (or the original concrete headers) so the types are available without forward declarations and adjust any include guards or dependencies accordingly.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cpp/include/cuopt/linear_programming/constants.h`:
- Around line 117-130: The constants header is missing the config key for the
parameter defined as farkas_obj_dynamism_tol in diving_hyper_params.hpp; add a
matching string constant (e.g., CUOPT_MIP_HYPER_DIVING_FARKAS_OBJ_DYNAMISM_TOL)
to cpp/include/cuopt/linear_programming/constants.h next to the other
diving-related defines so the config loader can find
"mip_hyper_diving_farkas_obj_dynamism_tol"; ensure the macro name matches the
naming pattern used for other keys (CUOPT_MIP_HYPER_DIVING_*) and the string
matches the parameter name used in diving_hyper_params.hpp.
In `@cpp/src/branch_and_bound/branch_and_bound.cpp`:
- Around line 1754-1757: Guard the division by checking objective magnitudes
before computing obj_dyn: if original_lp_.max_abs_obj_coeff == 0 set
diving_settings.farkas_diving = 0 (all-zero objective -> treat as low dynamism),
else only compute obj_dyn = log10(max/min) when original_lp_.min_abs_obj_coeff >
0 (use a tiny epsilon if you prefer to treat near-zero as zero); if min <= 0
skip the log10 check (leave farkas_diving enabled) to avoid inf/NaN, and keep
the existing comparison against diving_settings.farkas_obj_dynamism_tol when
obj_dyn is valid. Ensure you reference original_lp_.max_abs_obj_coeff,
original_lp_.min_abs_obj_coeff, diving_settings.farkas_diving, and
diving_settings.farkas_obj_dynamism_tol in the fix.
In `@cpp/src/branch_and_bound/diving_heuristics.cpp`:
- Around line 292-299: The Farkas score uses the wrong post-branch distances:
when dir == branch_direction_t::UP the branch bounds fix x_j >=
std::ceil(solution[j]) so compute remaining distance as lp.upper[j] -
std::ceil(solution[j]) (not lp.upper[j] - std::floor(solution[j])), and when dir
== branch_direction_t::DOWN use std::floor(solution[j]) - lp.lower[j] (not
std::ceil(solution[j]) - lp.lower[j]); update the calculation of score (the
block around score, lp.upper, lp.lower, solution[j], f_up, f_down, c, and f_t)
to use these post-branch bounds and keep the infinity fallback for non-finite
bounds.
In `@cpp/src/math_optimization/solver_settings.cu`:
- Line 175: The current option for CUOPT_MIP_HYPER_DIVING_BACKTRACK_LIMIT
exposes a raw upper bound of std::numeric_limits<i_t>::max(), but code
calculating the DFS stack uses (diving_backtrack_limit + 4) and can overflow;
change the option's maximum to a safe cap such as
(std::numeric_limits<i_t>::max() - 4) (or define a named constant like
MAX_SAFE_BACKTRACK = std::numeric_limits<i_t>::max() - 4) and use that instead
for the max value of mip_settings.diving_params.backtrack_limit so adding 4
cannot overflow.
---
Nitpick comments:
In `@cpp/src/branch_and_bound/pseudo_costs.hpp`:
- Around line 32-37: The forward declarations of branch_and_bound_worker_t and
branch_and_bound_stats_t in pseudo_costs.hpp violate the header policy; replace
these forward declarations by including the concrete headers that define
branch_and_bound_worker_t and branch_and_bound_stats_t (or refactor by
extracting the minimal strategy utilities used by pseudo_costs.hpp into a
lightweight header that both this file and the full worker/stats headers can
include). Update pseudo_costs.hpp to `#include` the new/lightweight header (or the
original concrete headers) so the types are available without forward
declarations and adjust any include guards or dependencies accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: b33259e4-7a93-4f0f-9856-e8d5aea35845
📒 Files selected for processing (15)
cpp/include/cuopt/linear_programming/constants.hcpp/include/cuopt/linear_programming/mip/diving_hyper_params.hppcpp/include/cuopt/linear_programming/mip/heuristics_hyper_params.hppcpp/include/cuopt/linear_programming/mip/solver_settings.hppcpp/src/branch_and_bound/branch_and_bound.cppcpp/src/branch_and_bound/constants.hppcpp/src/branch_and_bound/diving_heuristics.cppcpp/src/branch_and_bound/diving_heuristics.hppcpp/src/branch_and_bound/pseudo_costs.hppcpp/src/branch_and_bound/worker.hppcpp/src/dual_simplex/presolve.hppcpp/src/dual_simplex/simplex_solver_settings.hppcpp/src/math_optimization/solver_settings.cucpp/src/mip_heuristics/solver.cucpp/tests/mip/heuristics_hyper_params_test.cu
Signed-off-by: Nicolas L. Guidotti <nguidotti@nvidia.com>
nguidotti
requested review from
akifcorduk,
aliceb-nv and
chris-maes
and removed request for
Kh4ster and
rg20
nguidotti
changed the title
akifcorduk
left a comment
akifcorduk
left a comment
There was a problem hiding this comment.
One minor comment, thanks for PR Nicolas!
| } | ||
|
|
||
| if (dir == branch_direction_t::UP) { | ||
| score = std::isfinite(lp.upper[j]) |
There was a problem hiding this comment.
Wouldn't infinite scores come up often? Do you think using the largest variables upper bound in the problem to compute the scores of those upper=inf variables make sense? That way, you would create more balanced score rather than always blindly selecting upper=inf
There was a problem hiding this comment.
lp.upper[j] stores the variable upper bound for the variable nan operating with infinity). The first variable will infinite score will be selected (we are iterating with increasing indexes).
There was a problem hiding this comment.
You mean to use the max. value of the variable upper bound instead of infinity? This seems doable. We could also use a sentinel for infinity (SCIP do that, I think inf = 1E20)
There was a problem hiding this comment.
No, what I mean is to have some comparability in the scores. When a variables upper bound is infinite, it is chosen regardless of the objective coefficient and fractionality impact. For this variable, I would use the upper_bound as largest of the upper bounds among all variables (rough heuristics to determine cardinality). Then, other variables might also have chance to be chosen.
There was a problem hiding this comment.
I think this goes against the idea of the paper: "From the primal point of view, the potential change in the objective function depends on how much a variable can be pushed until it reaches one of its bounds.".
If the bounds is infinite, then we can push the variable to infinite as well. We should prioritize it over the other variables to allow for cutting that specific branch as early in the tree as possible.
2 participants
Implemented Farkas [1] and vector length [Section 9.2.6, 2] diving heuristics. It includes #1364.
Farkas diving is designed to move the current LP relaxation in direction to a valid Farkas proof. It pushes all variables towards the so-called pseudo solution, in which each variable assumes the best bound with respect to its objective coefficient as the solution value. Generally, this is very optimistic and most of the time, it will violate the constraints of the problem. Yet, if this finds a feasible solution, then it is expected to be very good.
Vector length diving is tailored for set partitioning and set covering. It chooses a rounding that coverst the largest number of constraints with the smallest objective value deterioration.
Benchmark Results
MIPLIB2017, 10min, GH200
All
Vector length diving
Farkas diving
References
[1] J. Witzig and A. Gleixner, “Conflict-Driven Heuristics for Mixed Integer Programming,” Feb. 07, 2019, arXiv: arXiv:1902.02615. doi: 10.48550/arXiv.1902.02615.
[2] T. Achterberg, “Constraint Integer Programming,” PhD, Technischen Universität Berlin, Berlin, 2007. doi: 10.14279/depositonce-1634.
Checklist